Skip to content

Conversation

@phpmaps
Copy link

@phpmaps phpmaps commented Mar 27, 2021

This PR adds some SSL support to make the CLI a little more secure :)

With Basic Authentication set on an HTTPS WebDav server like: https://bla:bla@webdav.geo.com/bucket the following exception would be raised by the node-fetch module during file transfer.

Error

FetchError: request to https://bla:bla@webdav.geo.com/bucket/test.txt failed, reason: unable to verify the first certificate
    at ClientRequest.<anonymous> (/Users/bla/webdav-watch/node_modules/node-fetch/lib/index.js:1461:11)
    at ClientRequest.emit (events.js:200:13)
    at ClientRequest.EventEmitter.emit (domain.js:471:20)
    at TLSSocket.socketErrorListener (_http_client.js:410:9)
    at TLSSocket.emit (events.js:200:13)
    at TLSSocket.EventEmitter.emit (domain.js:471:20)
    at emitErrorNT (internal/streams/destroy.js:91:8)
    at emitErrorAndCloseNT (internal/streams/destroy.js:59:3)
    at processTicksAndRejections (internal/process/task_queues.js:84:9) {
  message: 'request to ' +
    'https://bla:bla@webdav.geo.com/bucket/test.txt failed, ' +
    'reason: unable to verify the first certificate',
  type: 'system',
  errno: 'UNABLE_TO_VERIFY_LEAF_SIGNATURE',
  code: 'UNABLE_TO_VERIFY_LEAF_SIGNATURE'
}

This PR includes new proposed usage:

Commands

webdav-watch watch --folder ./music --s false --remote https://bla:bla@webdav.geo.com/bucket

OR

webdav-watch watch --folder ./music --ssl false --remote https://bla:bla@webdav.geo.com/bucket

AND

webdav-watch watch --folder ./music --s true --remote https://bla:bla@webdav.geo.com/bucket

OR

webdav-watch watch --folder ./music --ssl true --remote https://bla:bla@webdav.geo.com/bucket

Command using config

webdav-watch watch --c ./config.json

Example Configs

{
    "remote": " https://bla:bla@webdav.geo.com/bucket",
    "folder": "./music",
    "ssl": false
}
{
    "remote": " https://bla:bla@webdav.geo.com/bucket",
    "folder": "./music",
    "ssl": true
}

Copy link
Owner

@zemd zemd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@phpmaps Sorry for sooo long response, for some reason I didn't get any notifications about your participation. If this PR is still valid for you I like your idea, just couple things should be changed and we are good to go.

Comment on lines +38 to +43
.option('ssl', {
alias: 's',
describe: 'Suppress ssl issues',
type: 'boolean',
default: false
})
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's name this option strict-ssl-verification and it's default should be true

Comment on lines +95 to +96
const httpsAgent = new https.Agent({ rejectUnauthorized: false });
const opts = argv.ssl ? { body: fileContents, agent: httpsAgent } : { body: fileContents };
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const httpsAgent = new https.Agent({ rejectUnauthorized: false });
const opts = argv.ssl ? { body: fileContents, agent: httpsAgent } : { body: fileContents };
const agent = new https.Agent({ rejectUnauthorized: !argv.strictSslVerification });
const opts = Object.assign({body: fileContents}, argv.strictSslVerification ? {} : { agent });

Comment on lines +35 to +36
"inquirer": "^5.1.0",
"https": "^1.0.0"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since we are in nodejs environment, we don't need this dependency

Suggested change
"inquirer": "^5.1.0",
"https": "^1.0.0"
"inquirer": "^5.1.0"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants